feat: add dependencyInterpretation for MPIJob resource customization#7721
feat: add dependencyInterpretation for MPIJob resource customization#7721Ahmad-Faraj wants to merge 2 commits into
Conversation
MPIJob had no dependencyInterpretation, so ConfigMaps, Secrets, ServiceAccounts and PVCs referenced from its pod templates were not propagated with the job. Mirror the TFJob script over spec.mpiReplicaSpecs (Launcher and Worker), with testdata. Signed-off-by: Ahmad Faraj <ahmedfrag4040@gmail.com>
karmada-bot
added
the
kind/feature
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces dependency interpretation for MPIJob resources within the resource interpreter. By enabling the propagation of referenced ConfigMaps, Secrets, ServiceAccounts, and PVCs, it ensures that MPIJobs maintain their required dependencies during cross-cluster operations, mirroring the existing functionality provided for TFJobs. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
karmada-bot
added
the
size/L
There was a problem hiding this comment.
Pull request overview
Adds built-in dependency interpretation for Kubeflow MPIJob (v2beta1) so Karmada can automatically propagate referenced ConfigMaps, Secrets, ServiceAccounts, and PVCs from Launcher/Worker pod templates, matching existing TFJob/PyTorchJob behavior.
Changes:
- Add
dependencyInterpretationLua script forMPIJobthat walksspec.mpiReplicaSpecsforLauncherandWorkerand collects pod-template dependencies viakube.getPodDependencies. - Add
interpretdependency-test.yamltestdata covering multiple dependency sources, empty specs, default ServiceAccount exclusion, andenvFromreferences.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/resourceinterpreter/default/thirdparty/resourcecustomizations/kubeflow.org/v2beta1/MPIJob/customizations.yaml | Adds dependencyInterpretation for MPIJob by collecting dependencies from Launcher/Worker pod templates. |
| pkg/resourceinterpreter/default/thirdparty/resourcecustomizations/kubeflow.org/v2beta1/MPIJob/testdata/interpretdependency-test.yaml | Adds test cases verifying correct dependency extraction behavior for MPIJob. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces dependency interpretation for MPIJob resources by adding a Lua script to extract dependencies from replica specs (Launcher and Worker) and includes corresponding test cases. The reviewer suggested dynamically iterating over the replica specs using pairs instead of hardcoding the replica types to improve robustness and maintain consistency with other parts of the codebase.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| -- Iterate all MPIJob replica types | ||
| local replicaTypes = {"Launcher", "Worker"} | ||
| for _, replicaType in ipairs(replicaTypes) do | ||
| local spec = desiredObj.spec.mpiReplicaSpecs[replicaType] | ||
| if spec ~= nil and spec.template ~= nil then | ||
| local deps = kube.getPodDependencies(spec.template, desiredObj.metadata.namespace) | ||
| if deps ~= nil then | ||
| for _, dep in ipairs(deps) do | ||
| table.insert(refs, dep) | ||
| end | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Instead of hardcoding the replica types as {"Launcher", "Worker"}, we can dynamically iterate over all keys in desiredObj.spec.mpiReplicaSpecs using pairs. This is more robust, future-proof, and aligns with how replica specs are processed in GetComponents (line 56) in the same file.
-- Iterate over all replica specs dynamically to extract dependencies
for _, spec in pairs(desiredObj.spec.mpiReplicaSpecs) do
if spec ~= nil and spec.template ~= nil then
local deps = kube.getPodDependencies(spec.template, desiredObj.metadata.namespace)
if deps ~= nil then
for _, dep in ipairs(deps) do
table.insert(refs, dep)
end
end
end
end
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7721 +/- ##
=======================================
Coverage 42.06% 42.06%
=======================================
Files 879 879
Lines 54831 54831
=======================================
Hits 23063 23063
Misses 30023 30023
Partials 1745 1745
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Ahmad Faraj <ahmedfrag4040@gmail.com>
Ahmad-Faraj
force-pushed
the
feat/mpijob-dependency-interpretation
branch
from
0fa9dfa to
6a15b8c
Compare
What type of PR is this?
/kind feature
What this PR does / why we need it:
MPIJob's resource customization has no dependencyInterpretation, so ConfigMaps, Secrets, ServiceAccounts and PVCs referenced from its pod templates are not propagated with the job. Add one mirroring the TFJob script over spec.mpiReplicaSpecs (Launcher and Worker), with testdata like the existing kinds.
Which issue(s) this PR fixes:
Fixes #7720
Special notes for your reviewer:
Test cases follow the TFJob interpretdependency-test.yaml conventions and pass in TestThirdPartyCustomizationsFile.
Does this PR introduce a user-facing change?: